Fix #250: Replace form type object with classname string #253
Conversation
|
@bocharsky-bw thanks for this fix ... but I'm afraid I have bad news for you 😢 This project is compatible with |
|
For now, but when we update to the Symfony 3 we should use |
|
Yes. |
|
So keep it as is or update with fully qualified classname? |
|
Good question. I'd prefer to use the full string because that works perfectly on 2.8 and 3.0 ... but in the future we'll need to change it again to make code more concise. |
|
@bocharsky-bw, you forgot to update form type's classes too. See https://github.com/symfony/symfony-demo/blob/master/src/AppBundle/Form/PostType.php |
bocharsky-bw
force-pushed
the
form-type
branch
from
f98b4c3 to
6b35845
Compare
|
@javiereguiluz You're right. I updated with fully qualified classname for now. |
|
@voronkovich Did you mean this https://github.com/symfony/symfony-demo/blob/master/src/AppBundle/Form/PostType.php#L64 ? Seems for now we should use fully qualified classnames, but thanks you pointed it! |
|
@bocharsky-bw, ok. |
|
You should also switch to FQCN rather than legacy type names inside the existing form types to avoid deprecation warnings there |
|
@javiereguiluz this requires bumping the Symfony requirement to |
|
Switched. Thanks @stof ! |
| @@ -31,7 +31,7 @@ services: | |||
| app.form.type.datetimepicker: | |||
There was a problem hiding this comment.
@bocharsky-bw now we needn't this service. You can remove it.
There was a problem hiding this comment.
But why? It still used in https://github.com/bocharsky-bw/symfony-demo/blob/form-type/src/AppBundle/Form/PostType.php#L52-L54
There was a problem hiding this comment.
@bocharsky-bw, because in the PostType we use FQCN.
There was a problem hiding this comment.
Hm, it's true. Thanks!
|
This comment is not related to this PR. But IMHO these changes are a throwback for developers. :( |
| @@ -43,13 +43,13 @@ public function buildForm(FormBuilderInterface $builder, array $options) | |||
| 'attr' => array('autofocus' => true), | |||
| 'label' => 'label.title', | |||
| )) | |||
| ->add('summary', 'textarea', array('label' => 'label.summary')) | |||
| ->add('content', 'textarea', array( | |||
| ->add('summary', 'Symfony\Component\Form\Extension\Core\Type\TextareaType', array('label' => 'label.summary')) | |||
There was a problem hiding this comment.
This summary field is a string type, why use textarea? or perhaps we should redefine it as text?
There was a problem hiding this comment.
I think it doesn't necessary to change it to text... Even with 255 length more convenient work with textarea.
There was a problem hiding this comment.
@bocharsky-bw then, when users to type a text with 256 characters or more will be truncated and this could be an unexpected result for the user. I suggest then use maxlength 255 for this widget.
There was a problem hiding this comment.
max_length is deprecated option
There was a problem hiding this comment.
@voronkovich I meant to the html attribute:
'attr' => array('maxlength' => 255)
…elds in post type
|
@bocharsky-bw thanks for working on this. I'm merging it because I want to release a Symfony 2.8-based demo app soon. Thanks! |
No description provided.